chore: refactor permission rules and add additional validation by jeffsmale90 · Pull Request #7844 · MetaMask/core

jeffsmale90 · 2026-02-04T22:49:27Z

Explanation

It's critical that the GatorPermissionController's Permission decoding logic is strict, and will not decode EIP-712 payload to a permission unless the payload exactly meets the expectations of that permission.

This PR refactors the permission rules to be more self contained, and self describing. This allows each permission type's decoding rules to be more thoroughly tested in isolation. Validation and decoding logic is combined, as decoding is an implicit part of the validation step.

This PR also adds explicit validation of the "implicit" caveats for each permission type (valueLte for ERC20 permissions, exactCalldata for native token permissions), where previously we were ensuring that they caveats exist, but not validating their terms.

References

Checklist

I've updated the test suite for new or updated code as appropriate
I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
I've communicated my changes to consumers by updating changelogs for packages I've changed
I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

High Risk
Changes the core permission-type identification and caveat decoding/validation path; this could cause previously accepted permission contexts to be rejected or misclassified if rule matching/term checks are incorrect.

Overview
Refactors permission decoding to be rule-driven and stricter. GatorPermissionsController.decodePermissionFromPermissionContextForOrigin now builds per-chain permission rules and selects a single matching rule via findRuleWithMatchingCaveatAddresses, then validates+decodes caveat terms through rule.validateAndDecodePermission (replacing identifyPermissionByEnforcers + getPermissionDataAndExpiry).

Adds explicit caveat-term validation per permission type. New rule modules validate enforcer term structure and constraints (e.g., ExactCalldataEnforcer must be 0x, ValueLteEnforcer must be zero, byte-length checks, positive periodAmount/periodDuration/startTime, ERC20 tokenAddress hex validation, and stream maxAmount > initialAmount). The test suite is reorganized to test rules in isolation and adds adversarial/negative cases; changelog updated accordingly.

^{Written by Cursor Bugbot for commit 79be687. This will update automatically on new commits. Configure here.}

packages/gator-permissions-controller/src/GatorPermissionsController.ts

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts

mj-kiwi · 2026-03-03T04:49:29Z

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.test.ts

+  });
+
+  it('rejects when startTime is 0', () => {
+    const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddddd' as Hex;


I think the valid 20-byte addresses should be 40 hex chars.

Suggested change

const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddddd' as Hex;

const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddd' as Hex;

I think it is 40 hex chars:

> "0xdddddddddddddddddddddddddddddddddddddddd".length 42

"0x" followed by 40 "d"s is 42 chars

mj-kiwi · 2026-03-03T04:50:33Z

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts

+  });
+
+  it('rejects when startTime is 0', () => {
+    const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddddd' as Hex;


Suggested change

const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddddd' as Hex;

const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddd' as Hex;

Lol, I counted those ds very carefully!

Actually, the existing string if 42 chars, which is 20 bytes, which is correct.

I added validation to the erc20 permission decoders to ensure that the token addresses are the correct length.

Actually, I didn't, because we can't validate individual component lengths (the bytes are concatenated), but we do have terms length validation for each permission type.

packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts

mj-kiwi · 2026-03-03T04:51:30Z

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.ts

+    splitHex(terms, [20, 32, 32, 32]);
+  const periodDuration = hexToNumber(periodDurationRaw);
+  const startTime = hexToNumber(startTimeRaw);
+


should we add periodAmount > 0 check here?

packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.test.ts

packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts

…ion to ensure that permission data invariants are not violated.

…ype is self-describing and can be more easily tested in isolation. Add validation and test coverage for each permission type.

Plus minor changes: - Remove redundant amendment to ChecksumCaveat type - Remove unused ValidateDecodedPermission type - Fixes controller tests that expected the controller to self-report GatorPermissionsSnap id - Make decode functions internal, and rename to align with public interface

…e ChecksumEnforcersByChainId type rather than explicitly declaring a new type

…ithout specifying snapId

…twice

- use metamask/utils isHexAddress instead of regex to validate addresses - use hexToBigInt to validate tokenPeriod instead of hexToNumber

…). Add missing initialAmount validation.

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts

MoMannn

isSubset is now dead code and can be removed

MoMannn · 2026-03-05T07:19:10Z

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.ts

+    );
+  }
+
+  if (periodAmountBigInt <= 0n) {


These can check only if === 0n. There are no negative numbers in solidity so its not possible for them to be less then 0.

true - they're being decoded from hex so there's no way to encode a negative

MoMannn · 2026-03-05T07:20:51Z

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.ts

+
+  const EXPECTED_VALUE_LTE_TERMS_BYTELENGTH = 32;
+
+  if (getByteLength(valueLteTerms) !== EXPECTED_VALUE_LTE_TERMS_BYTELENGTH) {


This check is redundant. The next if is validating valueLteTerms !== ZERO_32_BYTES which also validates the length.

good point!

MoMannn · 2026-03-05T07:28:25Z

packages/gator-permissions-controller/src/decodePermission/decodePermission.ts

+ *   const result = matchingRules[0].validateAndDecodePermission(caveats);
+ *   if (result.isValid) { ... result.expiry, result.data ... }
+ *
+ * getPermissionRuleMatchingCaveatTypes and getPermissionDataAndExpiry use these rules


I believe these functions got removed now?

I removed this comment entirely as it's not really relevant with the refactor.

- checks now check value === 0 instead of <= 0 because hex encoding is always >= 0 - remove redundant terms length check

packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts

…tion comparing maxAmount and initialAmount.

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts

mj-kiwi

LGTM

jeffsmale90 changed the title ~~Add adversarial tests for permission decoding. Add additional validation for token permission types.~~ chore: add adversarial tests for permission decoding. Add additional validation for token permission types. Feb 10, 2026

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch from 4cd5553 to df72ecd Compare March 2, 2026 03:27

jeffsmale90 changed the title ~~chore: add adversarial tests for permission decoding. Add additional validation for token permission types.~~ chore: refactor permission rules and add additional validation Mar 2, 2026

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch 3 times, most recently from a9450f5 to 8700f23 Compare March 2, 2026 21:34

jeffsmale90 marked this pull request as ready for review March 2, 2026 21:34

jeffsmale90 requested a review from a team as a code owner March 2, 2026 21:34

cursor bot reviewed Mar 2, 2026

View reviewed changes

packages/gator-permissions-controller/src/GatorPermissionsController.ts Show resolved Hide resolved

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts Show resolved Hide resolved

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch from 52df231 to e1cb91e Compare March 2, 2026 23:29

jeffsmale90 requested a review from a team as a code owner March 3, 2026 01:17

mj-kiwi reviewed Mar 3, 2026

View reviewed changes

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts Show resolved Hide resolved

mj-kiwi reviewed Mar 3, 2026

View reviewed changes

packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts Show resolved Hide resolved

mj-kiwi reviewed Mar 3, 2026

View reviewed changes

cursor bot reviewed Mar 4, 2026

View reviewed changes

packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts Outdated Show resolved Hide resolved

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts Show resolved Hide resolved

cursor bot reviewed Mar 4, 2026

View reviewed changes

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts Show resolved Hide resolved

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch from 9990e4f to 5ff2c48 Compare March 4, 2026 01:30

jeffsmale90 requested a review from mj-kiwi March 4, 2026 01:30

cursor bot reviewed Mar 4, 2026

View reviewed changes

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.test.ts Show resolved Hide resolved

packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts Show resolved Hide resolved

cursor bot reviewed Mar 4, 2026

View reviewed changes

packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts Outdated Show resolved Hide resolved

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts Show resolved Hide resolved

jeffsmale90 added 8 commits March 5, 2026 19:12

Add adversarial tests for permission decoding. Add additional validat…

2ed88c2

…ion to ensure that permission data invariants are not violated.

Refactor PermissionRule and rule generation so that each permission t…

bfb1bde

…ype is self-describing and can be more easily tested in isolation. Add validation and test coverage for each permission type.

Linting fixes

7c1ec70

Each permission rule now derives it's internal enforcers type from th…

926429e

…e ChecksumEnforcersByChainId type rather than explicitly declaring a new type

Add test to ensure that the GatorPermissionsController instantiates w…

af21adb

…ithout specifying snapId

Update GatorPermissionsController test to not instantiate controller …

5b8f5d8

…twice

Minor fixups:

25a1116

- use metamask/utils isHexAddress instead of regex to validate addresses - use hexToBigInt to validate tokenPeriod instead of hexToNumber

jeffsmale90 added 4 commits March 5, 2026 19:14

Add amountPerSecond validation to erc20tokenstream

b8886fe

Trim whitespace

a175e39

Add missing validation to nativeTokenStream

92eba5c

Remove unnecessary maxAmount validation (already indirectly validated…

4b81a92

…). Add missing initialAmount validation.

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch 3 times, most recently from 830e176 to feff8e1 Compare March 5, 2026 06:26

Don't inline the hexToBigInt when validating

c7ba989

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch from feff8e1 to c7ba989 Compare March 5, 2026 06:29

cursor bot reviewed Mar 5, 2026

View reviewed changes

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts Show resolved Hide resolved

MoMannn requested changes Mar 5, 2026

View reviewed changes

jeffsmale90 requested a review from MoMannn March 5, 2026 18:03

jeffsmale90 added 3 commits March 6, 2026 07:05

Remove isSubset and related tests

6c21872

Update rules:

e0bd78b

- checks now check value === 0 instead of <= 0 because hex encoding is always >= 0 - remove redundant terms length check

Remove redundant comment

41c7393

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch from be36041 to 41c7393 Compare March 5, 2026 18:06

cursor bot reviewed Mar 5, 2026

View reviewed changes

packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts Show resolved Hide resolved

Remove erroneous initialAmount must be > 0 validation. correct valida…

c6577ca

…tion comparing maxAmount and initialAmount.

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch from 2d3f8f7 to c6577ca Compare March 5, 2026 21:09

cursor bot reviewed Mar 5, 2026

View reviewed changes

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts Show resolved Hide resolved

Remove newline

79be687

cursor bot reviewed Mar 6, 2026

View reviewed changes

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts Show resolved Hide resolved

mj-kiwi approved these changes Mar 6, 2026

View reviewed changes

jeffsmale90 enabled auto-merge March 6, 2026 07:19

MoMannn approved these changes Mar 6, 2026

View reviewed changes

jeffsmale90 added this pull request to the merge queue Mar 6, 2026

Merged via the queue into main with commit cb37525 Mar 6, 2026
322 checks passed

jeffsmale90 deleted the chore/gator_permissions_decoding_adversarial_tests branch March 6, 2026 07:44

	const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddddd' as Hex;
	const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddd' as Hex;


		const EXPECTED_VALUE_LTE_TERMS_BYTELENGTH = 32;

		if (getByteLength(valueLteTerms) !== EXPECTED_VALUE_LTE_TERMS_BYTELENGTH) {

Uh oh!

Conversation

jeffsmale90 commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

References

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mj-kiwi Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MoMannn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mj-kiwi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jeffsmale90 commented Feb 4, 2026 •

edited

Loading

mj-kiwi Mar 3, 2026 •

edited

Loading